feat: implement the Parser function to support the drop table #133
feat: implement the Parser function to support the drop table #133Adez017 wants to merge 3 commits into
Conversation
WalkthroughExtend the SQL parser to convert sqlparser DROP statements for tables into Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for opening your first pull request in NexumDB!
We appreciate your contribution! Here's what happens next:
- ✅ Automated checks will run (CI, tests, linting)
- 👀 A maintainer will review your changes
- 💬 We may suggest some improvements or ask questions
- 🚀 Once approved, we'll merge your contribution!
Before we can merge:
- Make sure all CI checks pass
- Sign your commits with DCO (
git commit -s) - Address any review feedback
Check out our Contributing Guide for more details.
Thanks for making NexumDB better! 🙌
Adez017
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nexum_core/src/sql/parser.rs (1)
243-265:⚠️ Potential issue | 🟡 MinorToken-based parsing cannot handle table names containing whitespace.
split_whitespaceat line 226 breaks identifiers with embedded spaces (e.g.,DROP TABLE `my table`yields 4 tokens, matching neither the 3-token nor 5-token branch). This would silently fall through to sqlparser, which can parse it, but the sqlparserDroparm usesnames[0].to_string()instead ofclean_identifier, potentially returning a differently-formatted name.If whitespace-containing identifiers aren't supported by the engine, a brief comment noting this limitation would help future maintainers. Otherwise, removing these management-parser branches in favor of the sqlparser path (as noted above) would handle this correctly.
🤖 Fix all issues with AI agents
In `@nexum_core/src/sql/parser.rs`:
- Around line 581-594: The test test_parse_drop_table_sqlparser_syntax is a
duplicate of test_parse_drop_table and never exercises the sqlparser Drop arm;
either delete this redundant test or change its SQL to trigger the sqlparser
path (e.g., use "DROP TABLE users CASCADE" or "DROP TABLE schema.users") and
update assertions in the test_parse_drop_table_sqlparser_syntax function
accordingly so it validates the parsed form produced by the sqlparser Drop arm
(refer to Parser::parse, Statement::DropTable and the sqlparser Drop handling
added around the Drop arm).
- Around line 200-220: The SqlStatement::Drop arm in convert_statement
(producing Statement::DropTable via names[0].to_string()) duplicates and
inconsistently normalizes identifiers compared to the management parser which
uses clean_identifier and intercepts DROP TABLE earlier; pick one canonical
path: either remove the management-parser DROP TABLE handling and let
convert_statement's SqlStatement::Drop handle DROP TABLEs (update it to
normalize identifiers using clean_identifier for names[0] and honor if_exists),
or delete this SqlStatement::Drop arm and keep the management parser as the
single handler; also remove the duplicate test
test_parse_drop_table_sqlparser_syntax (keep test_parse_drop_table) so tests
match the chosen single parsing path.
aviralgarg05
left a comment
aviralgarg05
left a comment
There was a problem hiding this comment.
Pls fix the issues and failing workflow
2 participants
🚀 OSCG'26 Contributor Notice
NexumDB participates in the Open Source Contributor Games 2026! High-quality PRs earn you points, recognition, and networking opportunities. Please follow our contribution guidelines for maximum impact and ensure your submission meets OSCG quality standards.
Summary
Athough a PR had added the storage concerns solve for drop statement but yet the parser implementation is missing whihc will be full filled now
Closes #132
What Changed?
Why?
Type of Change
Testing
Local Testing Checklist
cargo fmt --all -- --check- Code formattingcargo clippy --workspace --all-targets -- -D warnings- Lintingcargo test --workspace -- --test-threads=1- All tests passcargo audit- No security vulnerabilitiespython -m ruff check nexum_ai/- Python linting (if applicable)python -m compileall nexum_ai- Python compilation (if applicable)Testing Details
Screenshots/Examples
Performance Impact
Security Considerations
Documentation
demo.sh(if applicable)Pre-Merge Checklist
main🎯 OSCG'26 Quality Standards
By submitting this PR, I confirm:
Additional Notes
Reviewers: Please check that all checkboxes are ticked before approving
Summary by CodeRabbit
New Features
Tests